Skip to content

[Docs] [HTTP] [Trusted proxies] Removed note about adding 127.0.0.1 #3926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

ureimers
Copy link

Q A
Doc fix? yes
New docs? no
Applies to 2.3+ (applies to 2.2 too but wasn't stated in the 2.2 docs anyway)
Fixed tickets none

Removed the note about the need to add 127.0.0.1 to the list of trusted proxies when using Symfony's reversed proxy.

I don't think that adding 127.0.0.1 to the list of trusted proxies is needed. The only place where Symfony uses the list of trusted proxies is in the FragmentListener (https://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/HttpKernel/EventListener/FragmentListener.php#L81) and there they are merged with the result of FragmentListener#getLocalIpAddresses() (https://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/HttpKernel/EventListener/FragmentListener.php#L96) which already returns 127.0.0.1 among the list of locally (and thus trusted) ips.

Removed the note about the need to add 127.0.0.1 to the list of trusted Proxies when using Symfony's reversed Proxy.
@weaverryan
Copy link
Member

This comes from #3778. I might not have looked into it close enough - we'll see! :)

@tcz - you added this note originally. Do you have some thoughts here?

Thanks!

@tcz
Copy link
Contributor

tcz commented Jun 11, 2014

Well have there been any changes to how subrequests get the client IP? If not, I think my change is still valid. See this SO answer: http://stackoverflow.com/a/16895859

@ureimers
Copy link
Author

I think it's missleading. The reverse proxy works perfectly fine without adding 127.0.0.1 to the list of trusted proxies because it's using the above mentioned FragmentListener which automatically adds 127.0.0.1 to the list of trusted IPs.
Only if you want to read the client IP from within a sub-request you need to add 127.0.0.1 (which is strange in the first place and has it's own issue: symfony/symfony#9292).

Edit: Replaced "from within an ESI-included fragment" with "from within a sub-request".

@stof
Copy link
Member

stof commented Jun 11, 2014

@ureimers If you don't add your local reverse proxy to the list, the detection of the client IP or the fact that the request is secure will fail, as it will consider the request between the proxy and the app, not the request sent by the client to the proxy, and forwarded to your app

@stof
Copy link
Member

stof commented Jun 11, 2014

@weaverryan this note should be kept (even though some apps may not be impacted if they forget it)

@ureimers
Copy link
Author

@stof Thank you for the clarification, I got that. But could you then please have a look at the referenced Symfony issue? Because I don't think that it's consistent that the reverse proxie's FragementListener trusts 127.0.0.1 while Request::getClientIp() doesn't.
And if that issue should pass the note can be removed, and everyone can just read the client IP without having to bother about trusted IPs in local enviroments.

@wouterj
Copy link
Member

wouterj commented Aug 15, 2014

I don't know exactly what's going on here, but I trust @stof, so I'm closing this issue. However, feel free to keep commenting on it, we can always reopen it!

@wouterj wouterj closed this Aug 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants